Fix OSS build - #1
Conversation
34b6dda to
cb5455f
Compare
|
Hi @mszabo-wikia, I have been following the developments of your branches closely. I want to thank you for fixing the bit rot on hhvm@next. I have given you a shout out on my GH. I have built hhvm from source, with your branch as a base, for my OSS CI and for use deployed projects. I have a change that might be good to include in your upstream, added support for
These commit links might become invalid when I rebase on a later change of yours and force push, so hereby also a textual description of the changes made. The first two remove I hereby give you, and anymore else reading this, permission to use these diffs, in whole or in part, with or without attribution, for any purpose. My employer is aware I contribute to open source and has carved out an explicit permission for me to contribute in my personal capacity. |
|
@hershel-theodore-layton Thank you, I appreciate it. There's indeed no need to exclude these extensions now that the squangle maintainer has fixed the OSS build issues that were blocking it a few months back :) Thanks for your patches. Strangely the stray test file didn't cause any build errors locally for me, but you're correct that it shouldn't be part of a production build either way. I opted to remove it from the source list so it should no longer cause issues. As for the failing CI here... my sources say that GitHub Actions was disabled over at the main repo due to my overly enthusiastic rebasing causing a budget overrun, which certainly was not part of the plan. Sadly the default actions runners don't have enough free disk space to accommodate an HHVM build by default, but there should be ways to fix this by removing cruft from the default runner (which however also requires containerizing only the package build step instead of the entire job, else any attempt at cleanup won't be able to remove anything from the runner proper). It'll take some fiddling, but should be doable. LMK if you have any questions. |
7055206 to
d575db5
Compare
Summary:
The `MicroLock` tests fail under libc++ with asan-abort `stack-buffer-overflow`.
Example:
```
SCARINESS: 32 (4-byte-read-stack-buffer-overflow)
#0 unsigned int std::__2::__cxx_atomic_load[abi:ue170004]<unsigned int>(std::__2::__cxx_atomic_base_impl<unsigned int> const*, std::__2::memory_order) libcxx/include/c++/v1/__atomic/cxx_atomic_impl.h:375
#1 std::__2::__atomic_base<unsigned int, false>::load[abi:ue170004](std::__2::memory_order) const libcxx/include/c++/v1/__atomic/atomic_base.h:60
facebook#2 folly::MicroLockBase<1000u, 0u>::try_lock() folly/MicroLock.h:319
facebook#3 SmallLocks_MicroLockTryLock_Test::TestBody() folly/synchronization/test/SmallLocksTest.cpp:314
```
The trouble is accessing the lock state as a 4-byte word, where the asan stack has allocated only 1 byte for the lock object - asan tracks this and aborts.
It is UB for `MicroLock` to perform atomic stores or atomic read-modify-write operations on the entire 4-byte aligned word containing the `MicroLock` which may be concurrent with other accesses to other bytes in the word. The 4-byte aligned word accesses are necessary in order to support using the `MicroLock` itself as a futex, on Linux platforms. The trouble is that Linux does not provide futex operations on all sizes of integer, only on 32-bit (4-byte) integers. If Linux were to provide futex operations on all sizes of integer, we would immediately switch to 8-bit (1-byte) integers here.
This diff fixes the UB and gets the tests passing under libc++ by switching to the `folly::ParkingLot`, as wrapped by `folly::atomic_notify_one` and `folly::atomic_wait` over `std::atomic<uint8_t>`, which indirects the 32-bit (4-byte) futex behind a sharded hashtable.
Alternatives are possible for getting the tests passing, although these alternatives would not fix the UB.
* We can disable sanitizers on even more functions, as well as switch to atomic builtins since the members of `std::atomic` are not marked as disabling the sanitizers.
* We can workaround just in the unit-tests by padding the on-stack `MicroLock` instances.
Reviewed By: praihan, ot, aary
Differential Revision: D76612658
fbshipit-source-id: 75e9f03e63c85c30c6201b9bce5cd303c9a2b530
d575db5 to
539ffa8
Compare
Summary: After recent Python 3.12 rollout we started seeing crashes, e.g. T223049180. Crash stack from core dumps points to tstate being null when `_PyErr_GetRaisedException` is called ``` * thread #1, name = 'server#native-m', stop reason = SIGSEGV: address not mapped to object (fault address=0x60) * frame #0: 0x00007f94039e0197 libpython3.12.so.1.0`_PyFrame_MakeAndSetFrameObject [inlined] _PyErr_GetRaisedException(tstate=0x0000000000000000) at errors.c:490 frame #1: 0x00007f94039e0197 libpython3.12.so.1.0`_PyFrame_MakeAndSetFrameObject [inlined] PyErr_GetRaisedException at errors.c:499 frame facebook#2: 0x00007f94039e0184 libpython3.12.so.1.0`_PyFrame_MakeAndSetFrameObject(frame=0x00007f940977f340) at frame.c:32 frame facebook#3: 0x00007f9403b9d3de libpython3.12.so.1.0`PyThreadState_GetFrame [inlined] _PyFrame_GetFrameObject(frame=<unavailable>) (.__uniq.305758459065587366612134685926705492046) at pycore_frame.h:213 frame facebook#4: 0x00007f9403b9d3d0 libpython3.12.so.1.0`PyThreadState_GetFrame(tstate=<unavailable>) at pystate.c:1754 ``` The problem happens when this code is called (https://fburl.com/26dhwv4v) ``` PyObject * PyErr_GetRaisedException(void) { PyThreadState *tstate = _PyThreadState_GET(); return _PyErr_GetRaisedException(tstate); } ``` Where `_PyThreadState_GET` is implemented as follows https://fburl.com/92hrrluf ``` /* Get the current Python thread state. This function is unsafe: it does not check for error and it can return NULL. The caller must hold the GIL See also PyThreadState_Get() and _PyThreadState_UncheckedGet(). */ static inline PyThreadState* _PyThreadState_GET(void) { #if defined(HAVE_THREAD_LOCAL) && !defined(Py_BUILD_CORE_MODULE) return _Py_tss_tstate; #else return _PyThreadState_GetCurrent(); #endif ``` **The caller must hold the GIL** is a problem for us, since the whole idea behind implementing this hook this way is to get the stack when we **don't** have the GIL. This code implements a copy of the current To work around the issue we are copying the Python runtime code, but WITHOUT the GIL checks [_PyFrame_MakeAndSetFrameObject](https://github.com/python/cpython/blob/3.12/Python/frame.c#L28C1-L64C1) is reimplemented to skip the `PyErr_GetRaisedException` call, e.g. [_PyFrame_New_NoTrack](https://github.com/python/cpython/blob/main/Objects/frameobject.c#L2107C1-L2125C2) Changes from ``` PyFrameObject* _PyFrame_New_NoTrack(PyCodeObject *code) { CALL_STAT_INC(frame_objects_created); int slots = code->co_nlocalsplus + code->co_stacksize; PyFrameObject *f = PyObject_GC_NewVar(PyFrameObject, &PyFrame_Type, slots); if (f == NULL) { return NULL; } f->f_back = NULL; f->f_trace = NULL; f->f_trace_lines = 1; f->f_trace_opcodes = 0; f->f_lineno = 0; f->f_extra_locals = NULL; f->f_locals_cache = NULL; f->f_overwritten_fast_locals = NULL; return f; } ``` to ``` thread_local PyFrameObject my_frame; PyFrameObject* MyPyFrame_New_NoTrack(_PyInterpreterFrame* frame) { PyFrameObject* f = &my_frame; if (f == NULL) { return NULL; } f->f_back = NULL; f->f_trace = NULL; f->f_trace_lines = 1; f->f_trace_opcodes = 0; f->f_fast_as_locals = 0; f->f_lineno = 0; f->f_frame = frame; frame->frame_obj = f; Py_SET_TYPE(reinterpret_cast<PyObject*>(frame->frame_obj), &PyFrame_Type); return f; } ``` A thread local variable is being reused since we don't want to keep allocating and deallocating on the heap while doing heap profiling. Reviewed By: fried Differential Revision: D74118996 fbshipit-source-id: 65b9a9a10d6aa26f9c1cd93e30486333f20f4b24
Summary: This diff addresses a problem affecting the interaction of the OCaml runtime, Tokio, and Folly: - Our own `ocamlrep` crate allows us to register finalizers for the `Custom<>` types owned by OCaml. Usually, these just call `drop` on the Rust side. - When an OCaml program terminates, it does not call the finalizers of the objects that are alive at that point (see https://ocaml.org/docs/garbage-collection#finalisation-and-the-weak-module) - When running Hack with `edenfs_file_watcher_enabled`, the server env contains an (opaque) `Custom<EdenfsWatcherInstanceHandle>`, which owns the Tokio runtime executing a worker thread. Due to the points above, when calling `Exit.exit` in the server, the Tokio runtime and worker thread continue running. - Folly registers a low-level hook that is run when the program actually terminates (after the OCaml runtime has finished its own shutdown logic) - This low-level hook can deadlock when the `EdenfsWatcherInstance` is still running. I'm not sure what exactly is causing the deadlock (the fact that the worker thread is still running or the mere existence of the Tokio runtime, ...) Concretely, this problems manifests itself by the server sometimes getting stuck when calling `Exit.exit`. This diff changes the initialization code for `Edenfs_watcher` instances such that whenever we create an instance, we call `Exit.add_hook_upon_clean_exit` to register a hook that will properly shut down the instance. Note that this hook mechanism is part of our own code, and is run before the lower-level exit handler installed by Folly. ## Alternatives considered: OCaml has a "cleanup_on_exit mode, which among other things should call all finalizers (see https://ocaml.org/manual/5.3/runtime.html#s:ocamlrun-options). However, it seems to be buggy in OCaml 5.x (see ocaml/ocaml#10865 (comment)) and running hh_server with `OCAMLRUNPARAM=c` doesn't fix our problem. I'm not sure if we would want to use it anyway, as it may slow down server restarts. # Facebook Here's a stack trace where we get stuck. I've obtained it by attaching gdb to the server process: ``` #0 __futex_abstimed_wait_common64 (private=<optimized out>, cancel=true, abstime=0x0, op=265, expected=3432193, futex_word=0x7fffb4000910) at futex-internal.c:57 #1 __futex_abstimed_wait_common (cancel=true, private=<optimized out>, abstime=0x0, clockid=<optimized out>, expected=3432193, futex_word=0x7fffb4000910) at futex-internal.c:87 facebook#2 __GI___futex_abstimed_wait_cancelable64 (futex_word=0x7fffb4000910, expected=3432193, clockid=<optimized out>, abstime=0x0, private=<optimized out>) at futex-internal.c:139 facebook#3 0x00007ffff729c793 in __pthread_clockjoin_ex (threadid=140736213288512, thread_return=0x0, clockid=0, abstime=0x0, block=<optimized out>) at pthread_join_common.c:105 facebook#4 0x00007ffff7cdf84f in __gthread_join (__value_ptr=0x0, __threadid=<optimized out>) at /home/engshare/third-party2/libgcc/11.x/src/gcc-11.x/x86_64-facebook-linux/libstdc++-v3/include/x86_64-facebook-linux/bits/gthr-default.h:669 facebook#5 std::thread::join (this=0x7fffbe209110) at ../../../.././libstdc++-v3/src/c++11/thread.cc:112 facebook#6 0x00000000062a810d in folly::ThreadPoolExecutor::joinStoppedThreads(unsigned long) () facebook#7 0x00000000062a89ca in folly::ThreadPoolExecutor::stopAndJoinAllThreads(bool) () facebook#8 0x00000000062a395b in folly::IOThreadPoolExecutor::~IOThreadPoolExecutor() () facebook#9 0x00000000062a1975 in folly::IOThreadPoolExecutor::~IOThreadPoolExecutor() () facebook#10 0x0000000006452c09 in folly::detail::SingletonHolder<folly::IOThreadPoolExecutor>::destroyInstance() () facebook#11 0x0000000006fa3c0c in folly::SingletonVault::destroyInstances() () facebook#12 0x00007ffff72478b8 in __run_exit_handlers (status=4, listp=0x7ffff7412658 <__exit_funcs>, run_list_atexit=<optimized out>, run_dtors=<optimized out>) at exit.c:113 facebook#13 0x00007ffff72479ca in __GI_exit (status=<optimized out>) at exit.c:143 facebook#14 0x000000001f401e62 in caml_do_exit (retcode=4) at runtime/sys.c:200 facebook#15 0x000000001f4020dc in caml_sys_exit (retcode=<optimized out>) at runtime/sys.c:205 facebook#16 <signal handler called> facebook#17 0x000000001f33c3f8 in camlStdlib.exit_1534 () at stdlib.ml:580 facebook#18 0x000000001f030ec0 in camlExit.exit_24 () at fbcode/hphp/hack/src/utils/exit.ml:66 facebook#19 0x000000001c2bb13b in camlServerMain.exit_if_critical_update_249 () at fbcode/hphp/hack/src/server/serverMain.ml:108 facebook#20 0x000000001c2bb885 in camlServerMain.query_notifier_549 () at fbcode/hphp/hack/src/server/serverMain.ml:225 facebook#21 0x000000001c2bbe32 in camlServerMain.recheck_until_no_changes_left_916 () at fbcode/hphp/hack/src/server/serverMain.ml:331 ``` So we are shutting down the `folly::IOThreadPoolExecutor`, which then gets stuck waiting for something. A comment inside `__pthread_clockjoin_ex` at the point where we get stucks reads says ``` /* The kernel notifies a process which uses CLONE_CHILD_CLEARTID via futex wake-up when the clone terminates. The memory location contains the thread ID while the clone is running and is reset to zero by the kernel afterwards. The kernel up to version 3.16.3 does not use the private futex operations for futex wake-up when the clone terminates. */ ``` Differential Revision: D76737597 fbshipit-source-id: 979b4e9b3ae88a07fcf62f0958fe41372624d00b
d3e01b9 to
8e3b901
Compare
8e3b901 to
3e5305a
Compare
|
Thank you for fixing Same goes for including MCRouter in your branch with 423e8b1. This means I can remove this workaround too: if (!\extension_loaded('mcrouter')) {
return new ApcCache(); // cache-like wrapper around apcu
}
// Code below initializes cache-like wrapper around MCRouter, like I do on hhvm 4.168I also wanted to let you know I am publishing ready-to-use images to Docker Hub, based on your branch. I use them to run my CI. I am going to write about this on my GH page in July. I want to give you all the credit for doing the hard C++ and CMake work. I would like to include a quote of yours about the building of hhvm@next. I can accommodate anything from a single sentence to an entire above-the-fold section, if you'd want to. Here is a quick unrelated Bonus Tip. I don't know if you have already skip Lintian locally. This single threaded part might make up a significant part of your build time. You can disable lintian with this one-line change in make-debianish-package or with this ENV var from debuild. 🙂 |
Summary:
Updates typing of function references to polymorphic static methods so that we obtain a polymorphic function type. In particular the typing deals with the following:
- Use of method-level generics
- Use of class-level generics
- `where` constraints
- Use of `this`
- Use of abstract type constants
## Method-level generics
The current type of function references will not return a polymorphic function type. Instead, when referencing a polymorphic method the typing introduces type variables and adds bounds corresponding to any constraints on the generic it replaces. Given the declaration of `bar`:
```[hack]
class Wibble<Tclasslevel as arraykey> {
public static function bar<Tmethodlevel as arraykey>(
Tmethodlevel $y,
): void {}
}
function refEm(): void {
$fptr = Wibble::bar<>;
}
```
a fresh type variable will be created with an upper bound of `arraykey` and `$fptr` will be given the type
```
readonly function(#1 $x): #1
```
where the type variable `#1` has an upper-bound of `arraykey`. Using `$fptr` will be introduce additional constraints on the type variables so the following is currently ill-typed:
```[hack]
function expect_int_int((function(int): int) $f): void {}
function refEm(): (function(string): string) {
$fptr = Wibble::bar<>;
expect_int_int($fptr);
return $fptr;
}
```
After passing `$fptr` to `expect_int_int` the type variable will be solved to `int` so at the return expression we encounter
```
(function(int): int </: function(string): string)
```
With the new typing contained in this diff the function reference will instead be given a polymorphic function type:
```
HH\FunctionRef<(readonly function<Tmethodlevel as arraykey>(Tmethodlevel): Tmethodlevel)>
```
Subtyping of polymorphic function types permits us to pass this function where an expression of type `(function(int): int)` is expected. Doing so does not change the type and we are free to return `$fptr` where an expression of type `(function(string): string)` is expected.
Note that this typing is consistent with the already landed typing of generic top-level functions.
## Class-level generics
If a static method signature makes use of a generic declared in its enclosing class, it must be moved to the function type. For example:
```[hack]
class Wibble<Tclasslevel as arraykey> {
public static function foo(Tclasslevel $x): Tclasslevel {
return $x;
}
}
function refEm(): (function(string): string) {
$fptr = Wibble::foo<>;
return $fptr;
}
```
The expression `$fptr` will be given the polymorphic function type:
```
HH\FunctionRef<(readonly function<Tclasslevel as arraykey>(Tclasslevel $x): Tclasslevel)>
```
## `where` constraints
A `where` constraint expresses a restriction on a type parameter which may not be defined at the method level e.g.
```
class C<T> {
public static function foo(): void where T as arraykey {}
}
function refEm(): void {
$fptr = C::foo<>;
}
```
Since `where ` constraints aren't part of the function type, we simplify then discharge any such contraints onto type parameters. This is always possible since we have closed over any required class-level type parameters; in the example we then have:
```
$fptr: HH\FunctionRef<(readonly function<T as arraykey>(): void)>
```
`where` constraints can also relate method-level generics to class-level generics and is further complicated by inheritance.
```
class Base<T> {
public static function foo<Tu>(Tu $arg): T where T super vec<Tu> {
return vec[$arg];
}
}
final class Derived<Ta> extends Base<vec<Ta>> {}
function refEm(): void {
$fptr = Derived::foo<>;
}
```
```
$fptr: HH\FunctionRef<(readonly function<Ta super Tu, Tu as Ta>(Tu $arg): vec<Ta>)>
```
Note that this typing is consistent with the already landed typing of generic top-level functions.
## `this`
The type identifier `this` is used to refer to instance types of the lexically enclosing class. The type of function references to static methods which have `this` in their method signature will depend on the variance at which `this` occurs in the method signature. If `this` appears only contravariantly or covariantly, we can directly substitute the
`this` for the instance type of the enclosing class, for example
```[hack]
class Wibble<Tclasslevel as arraykey> {
public static function contra_only(this $_): void{}
public static function co_only(): this { ... }
}
function refEm(): void {
$fptr = Wibble::contra_only<>;
$gptr = Wibble::co_only<>;
}
```
```
$fptr : HH\FunctionRef<(readonly function<Tclasslevel as arraykey>(Wibble<Tclasslevel> $_): void)>
$gptr : HH\FunctionRef<(readonly function<Tclasslevel as arraykey>(): Wibble<Tclasslevel>)>
```
Where `this` occurs invariantly we introduce a type parameter:
```[hack]
class Wibble<Tclasslevel as arraykey> {
public static function invariantly(this $_): this { ... }
}
function refEm(): void {
$fptr = Wibble::invariantly<>;
}
```
```
$fptr: HH\FunctionRef<(readonly function<Tthis as Wibble<Tclasslevel>, Tclasslevel as arraykey>(Tthis $x): Tthis)>
```
Note that the variance of occurrences of `this` is determined by an analysis of the function signature and is 'aware' of the variance of type parameters in class declarations. In this example,
- `this` appears invariantly in the signature of `fizz` since `Box` is invariant in its type parameter
- `this` appears covariantly in the signature of `buzz` since `Contra` is contravariant in its type parameter but occurs in a contravariant position whilst `Co` is covariant in its type parameter and occurs in a covariant position.
```[hack]
class Box<T> {}
class Contra<-T> {}
class Co<+T> {}
class Wibble<Tclasslevel as arraykey> {
public static function fizz(Box<this> $_): void {}
public static function buzz(Contra<this> $_): ?Co<this>{
return null;
}
}
```
The logic for this part of typing is contained in the module `Typing_extract_method.This_variance`.
## Type constants
Abstract type constants and concrete type constant aliasing abstract type constants significantly complicate the typing of function references to static methods. The logic for this part of typing is contained in the module `Typing_extract_method.Typeconst_analysis`.
In order to 'extract' a method signature involving one of these constants from its containing class we make use of the following equivalence:
```
(exist T. U<T>) -> V === all T. (U<T> -> V)
```
To see how this applies, it's useful to think about the declaration of a class as being implicitly existentially quantified over all abstract type constants it contains. Moving these existentially quantified to type parameters and using class refinements to equate them allows us to be polymorphic in the type constant. For example:
```[hack]
abstract class Wibble {
abstract const type T as arraykey;
public static function foo(this $_, this::T $_, Wibble $_): void {}
}
function refEm(): void {
$fptr = Wibble::foo<>;
}
```
```
HH\FunctionRef<(readonly function<T#0 as arraykey>(Wibble with { type T = T#0 }, T#0, Wibble): void)>
```
Note that in this example, we are only refining the first parameter type (`this`), not the third (`Wibble`).
The refinement works for arbitrary access paths, for example
```[hack]
abstract class AbstractA {
abstract const type TA as arraykey;
}
abstract class AbstractLeft {
abstract const type TLeft as AbstractA;
}
abstract class AbstractRight {
abstract const type TRight as AbstractA;
}
abstract class AbstractB {
abstract const type TL as AbstractLeft;
abstract const type TR as AbstractRight;
}
abstract class AbstractC {
abstract const type TC as AbstractB;
public static function tickle(
this $_,
this::TC $_,
this::TC::TL $_,
this::TC::TR $_,
this::TC::TR::TRight $_,
this::TC::TL::TLeft $_,
this::TC::TR::TRight::TA $_,
this::TC::TL::TLeft::TA $_,
): void {}
}
function refEm(): void {
$fptr = AbstractC::tickle<>;
}
```
```
$fptr :
HH\FunctionRef<(readonly
function<
TA0 as arraykey,
TA1 as arraykey,
TC0 as AbstractB with { type TL = TL0; type TR = TR0 },
TL0 as AbstractLeft with { type TLeft = TLeft0 },
TLeft0 as AbstractA with { type TA = TA0 },
TR0 as AbstractRight with { type TRight = TRight0 },
TRight0 as AbstractA with { type TA = TA1 }
>(
AbstractC with { type TC = TC0 },
TC0,
TL0,
TR0,
TRight0,
TLeft0,
TA1,
TA0
): void)>
```
The analysis is also able to correctly type methods with abstract constants containing fix-point refinements:
```
abstract class AbstractA {
abstract const type T as AbstractA with { type T = this::T; };
public static function foo(this $_): this::T { throw new Exception(); }
}
function refEm(): void {
$fptr = AbstractA::foo<>;
hh_show($fptr);
}
```
```
$fptr:
HH\FunctionRef<(readonly
function<
T#0 as AbstractA with { type T = T#0 }
>(
AbstractA with { type T = T#0 }
): T#0)>
```
Reviewed By: andrewjkennedy
Differential Revision: D74076920
fbshipit-source-id: e4304f2680ac4b3b5d23c42759a867d2ee84b3f0
3e5305a to
84e2b01
Compare
|
@hershel-theodore-layton Thank you, I'm glad you find this useful :) I'm honestly not sure what to share about the actual build process. Maybe the single most thing I'm curious about is—who else is out there? AFAIK the largest HHVM users outside Meta are Slack (who have already reached out on the original PR and have been very helpful in hunting down IDE integration issues and other problems), Quizlet and Baidu, but the latter two might well have migrated to Zend at some point, so that leaves me wondering who else stayed on HHVM/Hack. Here follows an assortment of random notes that might be useful for others trying to build HHVM from scratch:
And some aspirational goals/improvements for the build system:
I'm very grateful to the folks from Meta who have been reviewing/landing my PRs split out from the original monstre PR when they find the time for it, so hopefully with time the main PR should get smaller and smaller. Once it's reasonably small, it might be interesting to setup some unofficial periodic OCI image build + publish workflow in a fork or dedicated repo so that people don't have to build their own if they don't want to. As always let me know if you have any questions/suggestions. |
You don't happen to have access to the old hacklang.slack.com Slack instance, right? This used to be the place that both enthusiasts and people who use HHVM at work could join. This is what the Google form at https://hhvm.com/slack was set up for. This Slack is inactive, but it hasn't been removed (yet). |
|
@hershel-theodore-layton Yeah, I came across the form but I never was in that Slack sadly |
Summary: Fix for issue #1 in Post [Truncation of UserMetric::Time](https://fb.workplace.com/groups/560979627394613/permalink/3320975644728317/) Our team uses [BENCHMARK_IMPL_COUNTERS](https://www.internalfb.com/code/fbsource/[f51516f2368d]/fbcode/folly/Benchmark.h?lines=379) to measure custom metrics during Folly::benchmarks execution. While working on updates to NodeAPI::SlaBenchmark in D77320464 I noticed Folly `UserMetric` accepts integer(Long) type values which works great for `UserMetric::Type::Custom` and `UserMetric::Type::Metric`, but causes issues for `UserMetric::Type::Time` values. When [time metrics](https://www.internalfb.com/code/fbsource/[f51516f2368d8097604a13de292824a7d85d7de1]/fbcode/folly/Benchmark.cpp?lines=529) are formated for display the value is implicitly cast(`Long` -> `Double`) resulting in loss of precision. This results in any value smaller than 1s being displayed as 0.00fs due to the truncation making it impossible to display small time values. ~~This diff adds support for users to provide floating point values as input to `UserMetric` allowing precision to be retained through a new `time_value` field, while remaining backward compatible with existing usage that relies on the existing `value` field.~~ See discussion below Reviewed By: yfeldblum Differential Revision: D77319941 fbshipit-source-id: a4d55165b5429fa602ea357764db03a81b9ac760
84e2b01 to
bafab28
Compare
1594e91 to
40dbaaf
Compare
Update and extend the list of hardening-related compiler flags used by HHVM to better represent modern distro defaults. * Convert the existing `ENABLE_SSP` build option into a new `ENABLE_HARDENING` option and put an updated list of security flags behind it. Both clang and GCC have been supporting these options for a while now, so we can set them irrespective of the compiler. * Put PIE-related options behind a separate `ENABLE_PIE` build option so that we can produce and compare non-PIE and PIE builds once we fix compatibility with PIE. * Forward `CMAKE_BUILD_TYPE` to the mcrouter extension. Lack of this was causing mcrouter to be built without compiler optimizations, which doesn't play well with `FORTIFY_SOURCE`. HHVM runs fine with these flags, requiring only a small fix to folly/Subprocess.cpp to fix compatibility with the overloaded `openat()` syscall from `FORTIFY_SOURCE`. The overhead from these flags is likely to be limited, as many of them have been set by default for distribution packages for several years now.[1] [1] https://github.com/jvoisin/compiler-flags-distro
libcxx removed all of the macro in favor simple feature test. See: llvm/llvm-project@040e9e0i Ported from facebook/folly#2404
Summary: Instruction CTZ is not available on armv9a CPUs. This implies that to compute trailing zeroes, RBIT followed by CLZ must be issued. We are changing the mask's logic to reverse bits once nad rely on CLZ instead of CTZ. The former instruction sequence looked like this: 2c3e48: fmov x15, d1 2c3e4c: rbit x16, x15 2c3e50: clz x16, x16 2c3e54: lsl x16, x16, #1 2c3e58: and x16, x16, #0xf8 2c3e5c: ldr w16, [x14, x16] 2c3e60: cbz w16, 2c3e88 <_ZN30F14Set_equalityRefinement_Test8TestBodyEv+0x35c> 2c3e64: sub x16, x15, #0x1 2c3e68: ands x15, x16, x15 2c3e6c: b.ne 2c3e4c <_ZN30F14Set_equalityRefinement_Test8TestBodyEv+0x320> // b.any The newer looks like this: 2c3d90: fmov x15, d1 2c3d94: rbit x15, x15 2c3d98: clz x17, x15 2c3d9c: lsl x18, x17, #1 2c3da0: and x18, x18, #0xf8 2c3da4: ldr w18, [x16, x18] 2c3da8: cbz w18, 2c3dd0 <_ZN30F14Set_equalityRefinement_Test8TestBodyEv+0x364> 2c3dac: lsr x17, x12, x17 2c3db0: ands x15, x17, x15 2c3db4: b.ne 2c3d98 <_ZN30F14Set_equalityRefinement_Test8TestBodyEv+0x32c> // b.any We can observe three things: -The final conditional branch jumps back to clz instead of rbit. -Instruction lsr depends on the result of clz, whereas sub depends on the result of rbit; meaning the old codepath can be executed speculatively earlier. -Assignment of 0x7FFFFFFFFFFFFFFF has been hoisted. There are no improvements nor regressions observed on benchmarks, probably it doesn't hit the case where many matches occur within the same tag. The added instruction of assigning 0x7FFFFFFFFFFFFFFF could potentially delay the tag memory load by 1 cycle, although it's unlikely This change allows performance improvements on occupiedIter: D94023144 Reviewed By: yfeldblum Differential Revision: D94020004 fbshipit-source-id: 8f8462f0290615fc8606e5ab9e3366a2a0733c51
Summary: X-link: facebook/folly#2589 The result of SparseMaskIter's next() is often used as an index on an 8-byte element array. In this case, the index needs to be shifted left by 3 to access the desired memory position. The return statement of the mentioned function contains i >> 2. The compiler is simplifying the shifts by only issuing a lsl 1 while ommitting the lsr 2. However, it then ANDs the shifted value by 0xf8, to ensure correctness when variable i is not a multiple of 4. We do know that variable i will always be a multiple of 4. We add the assume clause so the compiler avoids emitting the &0xf8 Before the assembly looked like this: clz x16, x16 lsl x16, x16, #1 and x16, x16, #0xf8 ldr x16, [x14, x16] After the changes, we verified the AND is omitted: clz x16, x16 lsl x16, x16, #1 ldr x16, [x14, x16] By removing a pipelined instruction in the codepath, execution latency is reduced by 1 cycle 🤗 It also allows the processor to foresee proceeding instructions up to 1 cycle earlier Reviewed By: yfeldblum Differential Revision: D94030304 fbshipit-source-id: c40a692345b051634c78c262eef8ee966a804104
Summary: `stack_epitaph()` captures the call stack and attaches it to `result` / `error_or_stopped` errors, with an optional user message. ## Motivation When a `result` coroutine's `unhandled_exception()` fires, the error says *what* was thrown but not *where*. This diff makes `unhandled_exception()` automatically attach a stack-trace epitaph: ``` std::runtime_error: boom [via] [stack: #0 coroThatThrows() @ stack_epitaph_test.cpp:70 #1 ...] ``` **Limitation**: The stack is captured after `throw` unwinds to the coroutine frame — you see which coroutine threw and its callers, but not the leaf that called `throw`. ## API `stack_epitaph()` wraps an error-or-stopped with a stack trace. Same semantics as `epitaph()` — transparent to `get_exception<T>()` and error codes, discarded on throw/eptr conversion, values pass through. ```cpp // Automatic in result coroutines (via unhandled_exception). // Public for user catch clauses: co_return stack_epitaph( error_or_stopped::from_current_exception(), "context"); ``` ## Costs - CPU: ~10ns/frame (libunwind walk). At ~20 frames, ~200ns — under 10% of throw+catch cost (~2us). - Memory: 192 bytes inline per epitaph. ~2KB transient stack for the capture buffer. ## Design `epitaph_non_value` is generalized into `epitaph_impl<Location>`, parameterized on location storage. Regular epitaphs use `epitaph_source_location` (zero-size via `[[no_unique_address]]`); stack epitaphs use `epitaph_stack_location<Opts>`. All underlying-pointer lifetime logic exists once. Stack frames use split storage: 17 frames inline (192 bytes = 3 cache lines), overflow in a ref-counted `shared_ptr<uintptr_t[]>`. When `inline_frames >= max_frames`, the heap member compiles to zero bytes. Capture uses `getStackTrace()` (libunwind); symbolization is deferred to format time. Reviewed By: janondrusek Differential Revision: D92859914 fbshipit-source-id: 51762e18b162105a4c424df633eaf2f81227e5a0
Summary: Previously, vasm trap is implemented with a `brk` on Arm64. However, `brk` triggers a SIGTRAP signal, which is not handled by HHVM's crash reporter code, which is the root cause for `hphp/test/slow/static-analysis-error.php` test failure. This diffs fixes the issue by implementing vasm trap on Arm64 with a `udf #1` instruction, which is Arm64's counterpart to x86's `ud2` instruction. `udf` triggers a SIGILL just like what `ud2` does. This fixes `hphp/test/slow/static-analysis-error.php` and achieves platform parity in terms of trap and crash handling. Reviewed By: binliu19 Differential Revision: D94627667 fbshipit-source-id: 16654bd424f1147e5fb1e7e336086dbf8cda612d
4c8bb58 to
aafaf27
Compare
aafaf27 to
9cca1c7
Compare
Running the HHBBC compiler against a trivial test program in recent OSS HHVM hangs forever. LLDB shows workers deadlocking in SubprocessScheduler. One thread is waiting on `m_subprocessSchedulingCV` called when a subprocess is released: ``` * thread facebook#5, name = 'HPHPcWorker2' * frame #0: 0x00007ffff436d9a2 libc.so.6`__syscall_cancel_arch_end frame #1: 0x00007ffff4361c3c libc.so.6`__internal_syscall_cancel + 92 frame facebook#2: 0x00007ffff43622ac libc.so.6`__futex_abstimed_wait_common + 124 frame facebook#3: 0x00007ffff436497e libc.so.6`pthread_cond_wait@@GLIBC_2.3.2 + 334 frame facebook#4: 0x00007ffff46e60f3 libc++.so.1`std::__1::condition_variable::wait(std::__1::unique_lock<std::__1::mutex>&) + 19 frame facebook#5: 0x000000000069dccc hhvm`HPHP::extern_worker::SubprocessScheduler::acquire(this=0x00007fffeea62980, jobName="hhbbc-build-subclass") at subprocess-scheduler.cpp:56:30 frame facebook#6: 0x00000000006593b8 hhvm`HPHP::extern_worker::(anonymous namespace)::SubprocessImpl::exec(this=<unavailable>, requestId=<unavailable>, command=<unavailable>, config=<unavailable>, inputs=<unavailable>, output=<unavailable>, finiOutput=<unavailable>, (null)=<unavailable>) (.resume) at extern-worker.cpp:1540:44 frame facebook#7: 0x00000000042d1891 hhvm`std::__1::coroutine_handle<void>::resume[abi:se210108](this=<unavailable>) const at coroutine_handle.h:69:5 [inlined] frame facebook#8: 0x00000000042d1881 hhvm`folly::resumeCoroutineWithNewAsyncStackRoot(h=coro frame = 0x7fffcc625000, frame=<unavailable>) at AsyncStack.cpp:202:5 frame facebook#9: 0x000000000061e3b0 hhvm`folly::coro::detail::co_reschedule_on_current_executor_::StackAwareAwaiter::await_suspend_impl(std::__1::coroutine_handle<void>, folly::AsyncStackFrame&)::'lambda'()::operator()(this=0x00007fffd5f75270) at CurrentExecutor.h:92:15 frame facebook#10: 0x00000000041f54a2 hhvm`folly::detail::function::FunctionTraits<void ()>::operator()(this=0x00007fffd5f75270) at Function.h:370:12 [inlined] frame facebook#11: 0x00000000041f5499 hhvm`void folly::catch_exception<folly::Function<void ()>&, void (&)(char const*) noexcept, char const*&, void>(t=0x00007fffd5f75270, c=<unavailable>, a=<unavailable>) at Exception.h:359:12 [inlined] frame facebook#12: 0x00000000041f5499 hhvm`void folly::Executor::invokeCatchingExns<folly::Function<void ()>>(p=<unavailable>, f=Function<void ()> @ 0x00007fffd5f75270) at Executor.h:233:5 [inlined] frame facebook#13: 0x00000000041f5499 hhvm`folly::ThreadPoolExecutor::runTask(this=0x00007fffee6cc800, thread=std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>::element_type @ 0x00007fffd9217140, task=0x00007fffd5f75410) at ThreadPoolExecutor.cpp:142:7 frame facebook#14: 0x00000000041d3c9b hhvm`folly::CPUThreadPoolExecutor::threadRun(this=0x00007fffee6cc800, thread=<unavailable>) at CPUThreadPoolExecutor.cpp:337:5 frame facebook#15: 0x00000000041f7533 hhvm`std::__1::__invoke_result_impl<void, void (folly::ThreadPoolExecutor::*&)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), folly::ThreadPoolExecutor*&, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>&>::type std::__1::__invoke[abi:se210108]<void (folly::ThreadPoolExecutor::*&)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), folly::ThreadPoolExecutor*&, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>&>(__args=<unavailable>, __args=<unavailable>, __args=<unavailable>) at invoke.h:87:27 [inlined] frame facebook#16: 0x00000000041f74fd hhvm`std::__1::__bind_return<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, std::__1::tuple<>, __is_valid_bind_return<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, std::__1::tuple<>>::value>::type std::__1::__apply_functor[abi:se210108]<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, 0ul, 1ul, std::__1::tuple<>>(__f=<unavailable>, __bound_args=<unavailable>, (null)=<unavailable>, __args=<unavailable>) at bind.h:195:10 [inlined] frame facebook#17: 0x00000000041f74fd hhvm`std::__1::__bind_return<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, std::__1::tuple<>, __is_valid_bind_return<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, std::__1::tuple<>>::value>::type std::__1::__bind<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>&>::operator()[abi:se210108]<>(this=<unavailable>) at bind.h:222:12 [inlined] frame facebook#18: 0x00000000041f74fd hhvm`void folly::detail::function::call_<std::__1::__bind<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>&>, true, false, void>(p=<unavailable>) at Function.h:341:5 frame facebook#19: 0x0000000000642d11 hhvm`folly::detail::function::FunctionTraits<void ()>::operator()(this=<unavailable>) at Function.h:370:12 [inlined] frame facebook#20: 0x0000000000642d0d hhvm`HPHP::(anonymous namespace)::ThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'()::operator()(this=0x00007fffd9254050) at coro.cpp:53:9 [inlined] frame facebook#21: 0x0000000000642cf2 hhvm`void folly::detail::function::call_<HPHP::(anonymous namespace)::ThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'(), false, false, void>(p=<unavailable>) at Function.h:341:5 frame facebook#22: 0x0000000000644770 hhvm`folly::detail::function::FunctionTraits<void ()>::operator()(this=<unavailable>) at Function.h:370:12 [inlined] frame facebook#23: 0x000000000064476d hhvm`folly::NamedThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'()::operator()(this=<unavailable>) at NamedThreadFactory.h:40:11 [inlined] frame facebook#25: 0x000000000064476d hhvm`void std::__1::__thread_execute[abi:se210108]<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, folly::NamedThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'()>(__t=size=2, (null)=<unavailable>) at thread.h:159:3 [inlined] frame facebook#26: 0x0000000000644749 hhvm`void* std::__1::__thread_proxy[abi:se210108]<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, folly::NamedThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'()>>(__vp=0x00007fffd9259150) at thread.h:168:3 frame facebook#27: 0x00007ffff4365464 libc.so.6`start_thread + 740 frame facebook#28: 0x00007ffff43e85ec libc.so.6`__clone3 + 44 ``` However this worker does that while also holding `m_subprocessMapLock`, so the other worker can't insert a subprocess: ``` * thread facebook#3, name = 'HPHPcWorker0' * frame #0: 0x00007ffff43624e0 libc.so.6`__GI___lll_lock_wait + 48 frame #1: 0x00007ffff4368a31 libc.so.6`__pthread_mutex_lock@GLIBC_2.2.5 + 257 frame facebook#2: 0x00007ffff46e724a libc++.so.1`std::__1::mutex::lock() + 10 frame facebook#3: 0x000000000069e074 hhvm`std::__1::lock_guard<std::__1::mutex>::lock_guard[abi:se210108](this=<unavailable>, __m=0x00007fffeea629e8) at lock_guard.h:32:10 [inlined] frame facebook#4: 0x000000000069e06c hhvm`HPHP::extern_worker::SubprocessScheduler::registerSubprocess(this=0x00007fffeea62980, pid=35418, jobName=<unavailable>) at subprocess-scheduler.cpp:75:31 frame facebook#5: 0x0000000000657a40 hhvm`HPHP::extern_worker::(anonymous namespace)::SubprocessImpl::doSubprocess(this=<unavailable>, requestId=<unavailable>, command=<unavailable>, inputBlob=<unavailable>, outputPath=<unavailable>, usageEstimate=<unavailable>) (.resume) at extern-worker.cpp:1657:28 frame facebook#6: 0x00000000042d1891 hhvm`std::__1::coroutine_handle<void>::resume[abi:se210108](this=<unavailable>) const at coroutine_handle.h:69:5 [inlined] frame facebook#7: 0x00000000042d1881 hhvm`folly::resumeCoroutineWithNewAsyncStackRoot(h=coro frame = 0x7ffff33c4000, frame=<unavailable>) at AsyncStack.cpp:202:5 frame facebook#8: 0x000000000061e3b0 hhvm`folly::coro::detail::co_reschedule_on_current_executor_::StackAwareAwaiter::await_suspend_impl(std::__1::coroutine_handle<void>, folly::AsyncStackFrame&)::'lambda'()::operator()(this=0x00007fffdf6f4270) at CurrentExecutor.h:92:15 frame facebook#9: 0x00000000041f54a2 hhvm`folly::detail::function::FunctionTraits<void ()>::operator()(this=0x00007fffdf6f4270) at Function.h:370:12 [inlined] frame facebook#10: 0x00000000041f5499 hhvm`void folly::catch_exception<folly::Function<void ()>&, void (&)(char const*) noexcept, char const*&, void>(t=0x00007fffdf6f4270, c=<unavailable>, a=<unavailable>) at Exception.h:359:12 [inlined] frame facebook#11: 0x00000000041f5499 hhvm`void folly::Executor::invokeCatchingExns<folly::Function<void ()>>(p=<unavailable>, f=Function<void ()> @ 0x00007fffdf6f4270) at Executor.h:233:5 [inlined] frame facebook#12: 0x00000000041f5499 hhvm`folly::ThreadPoolExecutor::runTask(this=0x00007fffee6cc800, thread=std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>::element_type @ 0x00007fffdff21240, task=0x00007fffdf6f4410) at ThreadPoolExecutor.cpp:142:7 frame facebook#13: 0x00000000041d3c9b hhvm`folly::CPUThreadPoolExecutor::threadRun(this=0x00007fffee6cc800, thread=<unavailable>) at CPUThreadPoolExecutor.cpp:337:5 frame facebook#14: 0x00000000041f7533 hhvm`std::__1::__invoke_result_impl<void, void (folly::ThreadPoolExecutor::*&)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), folly::ThreadPoolExecutor*&, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>&>::type std::__1::__invoke[abi:se210108]<void (folly::ThreadPoolExecutor::*&)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), folly::ThreadPoolExecutor*&, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>&>(__args=<unavailable>, __args=<unavailable>, __args=<unavailable>) at invoke.h:87:27 [inlined] frame facebook#15: 0x00000000041f74fd hhvm`std::__1::__bind_return<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, std::__1::tuple<>, __is_valid_bind_return<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, std::__1::tuple<>>::value>::type std::__1::__apply_functor[abi:se210108]<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, 0ul, 1ul, std::__1::tuple<>>(__f=<unavailable>, __bound_args=<unavailable>, (null)=<unavailable>, __args=<unavailable>) at bind.h:195:10 [inlined] frame facebook#16: 0x00000000041f74fd hhvm`std::__1::__bind_return<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, std::__1::tuple<>, __is_valid_bind_return<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, std::__1::tuple<>>::value>::type std::__1::__bind<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>&>::operator()[abi:se210108]<>(this=<unavailable>) at bind.h:222:12 [inlined] frame facebook#17: 0x00000000041f74fd hhvm`void folly::detail::function::call_<std::__1::__bind<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>&>, true, false, void>(p=<unavailable>) at Function.h:341:5 frame facebook#18: 0x0000000000642d11 hhvm`folly::detail::function::FunctionTraits<void ()>::operator()(this=<unavailable>) at Function.h:370:12 [inlined] frame facebook#19: 0x0000000000642d0d hhvm`HPHP::(anonymous namespace)::ThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'()::operator()(this=0x00007fffeeacb240) at coro.cpp:53:9 [inlined] frame facebook#20: 0x0000000000642cf2 hhvm`void folly::detail::function::call_<HPHP::(anonymous namespace)::ThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'(), false, false, void>(p=<unavailable>) at Function.h:341:5 frame facebook#21: 0x0000000000644770 hhvm`folly::detail::function::FunctionTraits<void ()>::operator()(this=<unavailable>) at Function.h:370:12 [inlined] frame facebook#22: 0x000000000064476d hhvm`folly::NamedThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'()::operator()(this=<unavailable>) at NamedThreadFactory.h:40:11 [inlined] frame facebook#24: 0x000000000064476d hhvm`void std::__1::__thread_execute[abi:se210108]<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, folly::NamedThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'()>(__t=size=2, (null)=<unavailable>) at thread.h:159:3 [inlined] frame facebook#25: 0x0000000000644749 hhvm`void* std::__1::__thread_proxy[abi:se210108]<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, folly::NamedThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'()>>(__vp=0x00007fffeea23d40) at thread.h:168:3 frame facebook#26: 0x00007ffff4365464 libc.so.6`start_thread + 740 frame facebook#27: 0x00007ffff43e85ec libc.so.6`__clone3 + 44 ``` So release `m_subprocessMapLock` in `acquire()` once we're done reading the map so that other threads get a chance to write it.
Running the HHBBC compiler against a trivial test program in recent OSS HHVM hangs forever. LLDB shows workers deadlocking in SubprocessScheduler. One thread is waiting on `m_subprocessSchedulingCV` called when a subprocess is released: ``` * thread facebook#5, name = 'HPHPcWorker2' * frame #0: 0x00007ffff436d9a2 libc.so.6`__syscall_cancel_arch_end frame #1: 0x00007ffff4361c3c libc.so.6`__internal_syscall_cancel + 92 frame facebook#2: 0x00007ffff43622ac libc.so.6`__futex_abstimed_wait_common + 124 frame facebook#3: 0x00007ffff436497e libc.so.6`pthread_cond_wait@@GLIBC_2.3.2 + 334 frame facebook#4: 0x00007ffff46e60f3 libc++.so.1`std::__1::condition_variable::wait(std::__1::unique_lock<std::__1::mutex>&) + 19 frame facebook#5: 0x000000000069dccc hhvm`HPHP::extern_worker::SubprocessScheduler::acquire(this=0x00007fffeea62980, jobName="hhbbc-build-subclass") at subprocess-scheduler.cpp:56:30 frame facebook#6: 0x00000000006593b8 hhvm`HPHP::extern_worker::(anonymous namespace)::SubprocessImpl::exec(this=<unavailable>, requestId=<unavailable>, command=<unavailable>, config=<unavailable>, inputs=<unavailable>, output=<unavailable>, finiOutput=<unavailable>, (null)=<unavailable>) (.resume) at extern-worker.cpp:1540:44 frame facebook#7: 0x00000000042d1891 hhvm`std::__1::coroutine_handle<void>::resume[abi:se210108](this=<unavailable>) const at coroutine_handle.h:69:5 [inlined] frame facebook#8: 0x00000000042d1881 hhvm`folly::resumeCoroutineWithNewAsyncStackRoot(h=coro frame = 0x7fffcc625000, frame=<unavailable>) at AsyncStack.cpp:202:5 frame facebook#9: 0x000000000061e3b0 hhvm`folly::coro::detail::co_reschedule_on_current_executor_::StackAwareAwaiter::await_suspend_impl(std::__1::coroutine_handle<void>, folly::AsyncStackFrame&)::'lambda'()::operator()(this=0x00007fffd5f75270) at CurrentExecutor.h:92:15 frame facebook#10: 0x00000000041f54a2 hhvm`folly::detail::function::FunctionTraits<void ()>::operator()(this=0x00007fffd5f75270) at Function.h:370:12 [inlined] frame facebook#11: 0x00000000041f5499 hhvm`void folly::catch_exception<folly::Function<void ()>&, void (&)(char const*) noexcept, char const*&, void>(t=0x00007fffd5f75270, c=<unavailable>, a=<unavailable>) at Exception.h:359:12 [inlined] frame facebook#12: 0x00000000041f5499 hhvm`void folly::Executor::invokeCatchingExns<folly::Function<void ()>>(p=<unavailable>, f=Function<void ()> @ 0x00007fffd5f75270) at Executor.h:233:5 [inlined] frame facebook#13: 0x00000000041f5499 hhvm`folly::ThreadPoolExecutor::runTask(this=0x00007fffee6cc800, thread=std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>::element_type @ 0x00007fffd9217140, task=0x00007fffd5f75410) at ThreadPoolExecutor.cpp:142:7 frame facebook#14: 0x00000000041d3c9b hhvm`folly::CPUThreadPoolExecutor::threadRun(this=0x00007fffee6cc800, thread=<unavailable>) at CPUThreadPoolExecutor.cpp:337:5 frame facebook#15: 0x00000000041f7533 hhvm`std::__1::__invoke_result_impl<void, void (folly::ThreadPoolExecutor::*&)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), folly::ThreadPoolExecutor*&, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>&>::type std::__1::__invoke[abi:se210108]<void (folly::ThreadPoolExecutor::*&)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), folly::ThreadPoolExecutor*&, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>&>(__args=<unavailable>, __args=<unavailable>, __args=<unavailable>) at invoke.h:87:27 [inlined] frame facebook#16: 0x00000000041f74fd hhvm`std::__1::__bind_return<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, std::__1::tuple<>, __is_valid_bind_return<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, std::__1::tuple<>>::value>::type std::__1::__apply_functor[abi:se210108]<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, 0ul, 1ul, std::__1::tuple<>>(__f=<unavailable>, __bound_args=<unavailable>, (null)=<unavailable>, __args=<unavailable>) at bind.h:195:10 [inlined] frame facebook#17: 0x00000000041f74fd hhvm`std::__1::__bind_return<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, std::__1::tuple<>, __is_valid_bind_return<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, std::__1::tuple<>>::value>::type std::__1::__bind<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>&>::operator()[abi:se210108]<>(this=<unavailable>) at bind.h:222:12 [inlined] frame facebook#18: 0x00000000041f74fd hhvm`void folly::detail::function::call_<std::__1::__bind<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>&>, true, false, void>(p=<unavailable>) at Function.h:341:5 frame facebook#19: 0x0000000000642d11 hhvm`folly::detail::function::FunctionTraits<void ()>::operator()(this=<unavailable>) at Function.h:370:12 [inlined] frame facebook#20: 0x0000000000642d0d hhvm`HPHP::(anonymous namespace)::ThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'()::operator()(this=0x00007fffd9254050) at coro.cpp:53:9 [inlined] frame facebook#21: 0x0000000000642cf2 hhvm`void folly::detail::function::call_<HPHP::(anonymous namespace)::ThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'(), false, false, void>(p=<unavailable>) at Function.h:341:5 frame facebook#22: 0x0000000000644770 hhvm`folly::detail::function::FunctionTraits<void ()>::operator()(this=<unavailable>) at Function.h:370:12 [inlined] frame facebook#23: 0x000000000064476d hhvm`folly::NamedThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'()::operator()(this=<unavailable>) at NamedThreadFactory.h:40:11 [inlined] frame facebook#25: 0x000000000064476d hhvm`void std::__1::__thread_execute[abi:se210108]<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, folly::NamedThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'()>(__t=size=2, (null)=<unavailable>) at thread.h:159:3 [inlined] frame facebook#26: 0x0000000000644749 hhvm`void* std::__1::__thread_proxy[abi:se210108]<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, folly::NamedThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'()>>(__vp=0x00007fffd9259150) at thread.h:168:3 frame facebook#27: 0x00007ffff4365464 libc.so.6`start_thread + 740 frame facebook#28: 0x00007ffff43e85ec libc.so.6`__clone3 + 44 ``` However this worker does that while also holding `m_subprocessMapLock`, so the other worker can't insert a subprocess: ``` * thread facebook#3, name = 'HPHPcWorker0' * frame #0: 0x00007ffff43624e0 libc.so.6`__GI___lll_lock_wait + 48 frame #1: 0x00007ffff4368a31 libc.so.6`__pthread_mutex_lock@GLIBC_2.2.5 + 257 frame facebook#2: 0x00007ffff46e724a libc++.so.1`std::__1::mutex::lock() + 10 frame facebook#3: 0x000000000069e074 hhvm`std::__1::lock_guard<std::__1::mutex>::lock_guard[abi:se210108](this=<unavailable>, __m=0x00007fffeea629e8) at lock_guard.h:32:10 [inlined] frame facebook#4: 0x000000000069e06c hhvm`HPHP::extern_worker::SubprocessScheduler::registerSubprocess(this=0x00007fffeea62980, pid=35418, jobName=<unavailable>) at subprocess-scheduler.cpp:75:31 frame facebook#5: 0x0000000000657a40 hhvm`HPHP::extern_worker::(anonymous namespace)::SubprocessImpl::doSubprocess(this=<unavailable>, requestId=<unavailable>, command=<unavailable>, inputBlob=<unavailable>, outputPath=<unavailable>, usageEstimate=<unavailable>) (.resume) at extern-worker.cpp:1657:28 frame facebook#6: 0x00000000042d1891 hhvm`std::__1::coroutine_handle<void>::resume[abi:se210108](this=<unavailable>) const at coroutine_handle.h:69:5 [inlined] frame facebook#7: 0x00000000042d1881 hhvm`folly::resumeCoroutineWithNewAsyncStackRoot(h=coro frame = 0x7ffff33c4000, frame=<unavailable>) at AsyncStack.cpp:202:5 frame facebook#8: 0x000000000061e3b0 hhvm`folly::coro::detail::co_reschedule_on_current_executor_::StackAwareAwaiter::await_suspend_impl(std::__1::coroutine_handle<void>, folly::AsyncStackFrame&)::'lambda'()::operator()(this=0x00007fffdf6f4270) at CurrentExecutor.h:92:15 frame facebook#9: 0x00000000041f54a2 hhvm`folly::detail::function::FunctionTraits<void ()>::operator()(this=0x00007fffdf6f4270) at Function.h:370:12 [inlined] frame facebook#10: 0x00000000041f5499 hhvm`void folly::catch_exception<folly::Function<void ()>&, void (&)(char const*) noexcept, char const*&, void>(t=0x00007fffdf6f4270, c=<unavailable>, a=<unavailable>) at Exception.h:359:12 [inlined] frame facebook#11: 0x00000000041f5499 hhvm`void folly::Executor::invokeCatchingExns<folly::Function<void ()>>(p=<unavailable>, f=Function<void ()> @ 0x00007fffdf6f4270) at Executor.h:233:5 [inlined] frame facebook#12: 0x00000000041f5499 hhvm`folly::ThreadPoolExecutor::runTask(this=0x00007fffee6cc800, thread=std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>::element_type @ 0x00007fffdff21240, task=0x00007fffdf6f4410) at ThreadPoolExecutor.cpp:142:7 frame facebook#13: 0x00000000041d3c9b hhvm`folly::CPUThreadPoolExecutor::threadRun(this=0x00007fffee6cc800, thread=<unavailable>) at CPUThreadPoolExecutor.cpp:337:5 frame facebook#14: 0x00000000041f7533 hhvm`std::__1::__invoke_result_impl<void, void (folly::ThreadPoolExecutor::*&)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), folly::ThreadPoolExecutor*&, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>&>::type std::__1::__invoke[abi:se210108]<void (folly::ThreadPoolExecutor::*&)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), folly::ThreadPoolExecutor*&, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>&>(__args=<unavailable>, __args=<unavailable>, __args=<unavailable>) at invoke.h:87:27 [inlined] frame facebook#15: 0x00000000041f74fd hhvm`std::__1::__bind_return<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, std::__1::tuple<>, __is_valid_bind_return<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, std::__1::tuple<>>::value>::type std::__1::__apply_functor[abi:se210108]<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, 0ul, 1ul, std::__1::tuple<>>(__f=<unavailable>, __bound_args=<unavailable>, (null)=<unavailable>, __args=<unavailable>) at bind.h:195:10 [inlined] frame facebook#16: 0x00000000041f74fd hhvm`std::__1::__bind_return<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, std::__1::tuple<>, __is_valid_bind_return<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), std::__1::tuple<folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>>, std::__1::tuple<>>::value>::type std::__1::__bind<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>&>::operator()[abi:se210108]<>(this=<unavailable>) at bind.h:222:12 [inlined] frame facebook#17: 0x00000000041f74fd hhvm`void folly::detail::function::call_<std::__1::__bind<void (folly::ThreadPoolExecutor::*)(std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>), folly::ThreadPoolExecutor*, std::__1::shared_ptr<folly::ThreadPoolExecutor::Thread>&>, true, false, void>(p=<unavailable>) at Function.h:341:5 frame facebook#18: 0x0000000000642d11 hhvm`folly::detail::function::FunctionTraits<void ()>::operator()(this=<unavailable>) at Function.h:370:12 [inlined] frame facebook#19: 0x0000000000642d0d hhvm`HPHP::(anonymous namespace)::ThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'()::operator()(this=0x00007fffeeacb240) at coro.cpp:53:9 [inlined] frame facebook#20: 0x0000000000642cf2 hhvm`void folly::detail::function::call_<HPHP::(anonymous namespace)::ThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'(), false, false, void>(p=<unavailable>) at Function.h:341:5 frame facebook#21: 0x0000000000644770 hhvm`folly::detail::function::FunctionTraits<void ()>::operator()(this=<unavailable>) at Function.h:370:12 [inlined] frame facebook#22: 0x000000000064476d hhvm`folly::NamedThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'()::operator()(this=<unavailable>) at NamedThreadFactory.h:40:11 [inlined] frame facebook#24: 0x000000000064476d hhvm`void std::__1::__thread_execute[abi:se210108]<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, folly::NamedThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'()>(__t=size=2, (null)=<unavailable>) at thread.h:159:3 [inlined] frame facebook#25: 0x0000000000644749 hhvm`void* std::__1::__thread_proxy[abi:se210108]<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, folly::NamedThreadFactory::newThread(folly::Function<void ()>&&)::'lambda'()>>(__vp=0x00007fffeea23d40) at thread.h:168:3 frame facebook#26: 0x00007ffff4365464 libc.so.6`start_thread + 740 frame facebook#27: 0x00007ffff43e85ec libc.so.6`__clone3 + 44 ``` So release `m_subprocessMapLock` in `acquire()` once we're done reading the map so that other threads get a chance to write it. SubprocessScheduler also sets the memory limit that may be used by subprocesses to 60% of the system RAM. Before scheduling a subprocess, it tries to check if the estimated memory usage of the subprocess will exceed this limit, and block until another subprocess has freed memory. It starts with an initial hardcoded 20GiB memory usage estimate that immediately deadlocks on systems with < ~32GiB RAM as we start blocking on `m_subprocessSchedulingCV` waiting for subprocesses to free memory before we even scheduled any. So avoid blocking if no subprocesses have been started yet, as we'd never be unblocked.
Historically, HHVM statically linked the system libbfd to power stack trace symbolization in its crash reports and in perf map files. This was disabled for OSS in D2137703[1] because of the wildly differing libbfd ABI across target systems. D3742004[2] and D3855027[3] then brought in folly::Symbolizer for both use cases, but this remained gated behind a Meta-only define. folly::Symbolizer has been stable in upstream folly for a while now, so let's enable it unconditionally and deshim the experimental/ header. This also allows removing the dead libbfd integration. For this to work, the folly build must be able to find libunwind, which isn't a given when building against libc++ and LLVM libunwind on Ubuntu as it installs includes into a subdirectory, so provide an additional hint for this case. This allows us to finally have a proper stacktrace rather than raw memory addresses in crash reports, such as: ``` Core dumped: Segmentation fault Stack trace in /tmp/stacktrace.902095.log Host: hhvm-jammy-vm ProcessID: 902095 ThreadID: 281473658339392 ThreadPID: 902095 Name: /usr/local/bin/hhvm CmdLine: hhvm ../hhvm/hphp/test/quick/dv.php Type: Segmentation fault Runtime: hhvm Version: heads/7.70.0-slack-0-g7be53eff8d22faa0383a128696fae83b224aedd5 DebuggerCount: 0 Arguments: ../hhvm/hphp/test/quick/dv.php ThreadType: CLI -------------------------------Treadmill Information---------------------------- Now: 235093382994348 OldestStartTime: 235093373066177 InflightRequestsSize: 1 Active Requests: 281473658339392 1 235093373066177 (age 9ms) (timeout 0s) CLISession OLDEST ------------------------------------------------------------------------------ CPP Stacktrace: # 0 HPHP::jit::FixupMap::processFixupForVMFrame(HPHP::jit::VMFrame) # 1 HPHP::jit::FixupMap::fixupWork(HPHP::ActRec*, bool) # 2 HPHP::jit::detail::syncVMRegsWork(bool) # 3 HPHP::createBacktrace(HPHP::BacktraceArgs const&) # 4 HPHP::f_debug_backtrace(long, long) # 5 HPHP::throwable_init(HPHP::ObjectData*) # 6 HPHP::ObjectData* HPHP::ObjectData::newInstance<false>(HPHP::Class*) # 7 HPHP::SystemLib::AllocRuntimeExceptionObject(HPHP::Variant const&) # 8 HPHP::SystemLib::throwRuntimeExceptionObject(HPHP::Variant const&) # 9 HPHP::throwMissingArgument(HPHP::Func const*, int) PHP Stacktrace: #0 main() called at [/home/mszabo/hhvm/hphp/test/quick/dv.php:28] #1 main_entry() ``` We can leverage this to augment coredumper to also provide an extract of the stack trace in its reports, instead of having to go through the rather onerous flow of downloading the coredump and attaching a debugger each time. [1] facebook@a66dd13 [2] facebook@c11ad4e [3] https://github.com/facebook/hhvm/commits/master/hphp/runtime/vm/debug/debug.cpp
…CallbackWithState
Summary:
What: Deliver `writeStarting()` to the `WriteCallback` exactly once per write on the native `AsyncSocket` + io_uring send path. `IoUringSend`'s `SendRequest` now holds the `WriteCallbackWithState`, and `onSendStarted()` calls `notifyOnWrite()` (which dedups via `writeInProgress_`); `AsyncSocket::writeImpl` passes its `callbackWithState` into `iouSendHandle_->write()` so an immediate write arrives already-notified.
Why: The io_uring send handle fired `writeStarting()` on every SQE submit while `AsyncSocket` also fired it via `notifyOnWrite()`, so a single write got two calls (and more on partial / event-re-registration re-submits). `RocketServerConnection::writeStarting()` DCHECKs that `startRawByteOffset` is unset, so the duplicate aborts the process (`RocketServerConnection.cpp:718`); in opt builds it silently corrupts the byte offset. This crashed graphstore once it served on native `AsyncSocket` over io_uring with zero-copy (D111176951).
```
Rocket flushWrites
-> AsyncSocket::writeImpl
|- notifyOnWrite() --------------> writeStarting() #1 (sets startRawByteOffset)
`- iouSendHandle_->write -> submit
`- onSendStarted() ---------> writeStarting() facebook#2 -> DCHECK abort
(+ again on every partial / re-arm re-submit)
fix: both paths funnel through one WriteCallbackWithState.notifyOnWrite() -> fires once
```
Rendered flow: https://pxl.cl/bBVG9
Reviewed By: spikeh
Differential Revision: D111316678
fbshipit-source-id: 037ff58303d0fbff4e8e77af6b3921f9463a7196
As facebook#9564, but with an attempt to get it to build on a plain GH Actions runner.